Fix/oauth jwt cookie - #153
Conversation
|
@dubemoyibe-star Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds Zod schemas for login and signup validation, switches Google OAuth callback token delivery to an httpOnly cookie, updates token extraction to read cookies, and expands tests for validation and cookie-based auth. ChangesAuth Validation and Cookie Token Delivery
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/validators/auth.validator.ts (1)
3-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer Zod 4’s documented
{ error: ... }form here.The PR context documents Zod 4’s unified
errorparameter, but these schemas still use positional string overloads. Switching now avoids relying on legacy signatures across all four validators.Proposed refactor
export const LoginSchema = z.object({ - email: z.string().email('Invalid email format'), - password: z.string().min(1, 'Password is required'), + email: z.string().email({ error: 'Invalid email format' }), + password: z.string().min(1, { error: 'Password is required' }), }); @@ export const SignupSchema = z.object({ - name: z.string().min(1, 'Name is required'), - email: z.string().email('Invalid email format'), - password: z.string().min(1, 'Password is required'), + name: z.string().min(1, { error: 'Name is required' }), + email: z.string().email({ error: 'Invalid email format' }), + password: z.string().min(1, { error: 'Password is required' }), });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/validators/auth.validator.ts` around lines 3 - 14, Update the Zod validators in LoginSchema and SignupSchema to use Zod 4’s documented { error: ... } option instead of positional string messages. Replace the current string arguments on email() and min() with the unified error form for all four field validators so the auth schema definitions align with the newer API and avoid legacy overloads.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/utils/helper.ts`:
- Around line 12-13: The token extraction logic in extractToken is too
permissive because it accepts any Authorization header with a second segment and
lets it override the cookie; update it so only a Bearer authorization header is
used before falling back to parseCookie(req.headers.cookie, 'token'). Keep the
existing extractToken behavior otherwise, but add an explicit scheme check in
the authorization parsing path so Basic and other non-Bearer headers do not
block a valid cookie token.
In `@src/validators/auth.validator.ts`:
- Around line 10-13: The SignupSchema name field currently uses
z.string().min(1), which still allows whitespace-only values to pass through
parsed.data.name. Update the name validator in SignupSchema to trim the input
before applying the requiredness check, so blank display names are rejected
consistently while keeping the existing validation message behavior.
In `@tests/auth.middleware.test.ts`:
- Around line 58-76: The invalid-cookie test is mocking JwtVerify with a plain
Error, which does not match the real verification failure shape used by
authGuard and handleAuthError. Update the test in auth.middleware.test.ts to
simulate the actual jwt.JsonWebTokenError contract from src/middlewares/jwt.ts
so the authGuard path is exercised as production handles it, and keep the
assertions aligned with the Unauthorized: Invalid token mapping.
---
Nitpick comments:
In `@src/validators/auth.validator.ts`:
- Around line 3-14: Update the Zod validators in LoginSchema and SignupSchema to
use Zod 4’s documented { error: ... } option instead of positional string
messages. Replace the current string arguments on email() and min() with the
unified error form for all four field validators so the auth schema definitions
align with the newer API and avoid legacy overloads.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 493416db-f4f9-4870-a0fe-104ba151af82
📒 Files selected for processing (8)
src/controllers/login.controller.tssrc/controllers/signup.controller.tssrc/routes/auth.route.tssrc/utils/helper.tssrc/validators/auth.validator.tstests/auth.middleware.test.tstests/login.controller.test.tstests/signup.controller.test.ts
|
@dubemoyibe-star pls resolve coderabbit requested changes |
|
@DioChuks |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…star/zicket-backend into fix/oauth-jwt-cookie
|
@DioChuks |
|
Well done. |
Summary
This PR fixes the insecure transmission of JWT tokens during the Google OAuth flow by removing the token from URL query parameters and replacing it with a secure delivery mechanism.
Closes #138
Problem
The Google OAuth callback previously redirected users to the frontend using:
Passing authentication tokens in URLs exposes them to browser history, server logs, proxy logs, analytics tools, and Referer headers, increasing the risk of token leakage.
Changes
Security Improvements
The authentication token is now:
Testing
Checklist
Summary by CodeRabbit
Validation failedresponses.httpOnlycookie and redirects without exposing the token in the URL.